Conversation
…t the suite
`nodedb-codec` gates its codec by target already: C libzstd off wasm, ruzstd on
it (`Cargo.toml:31-36`), and `compress_native`'s wasm arm is an explicit stub
returning `CompressFailed` (`src/zstd_codec.rs:312-320`). The eight tests that call
`encode`/`encode_with_level` therefore panic, and on
`wasm32-wasip1` a panic aborts the process: the binary dies at the first one and
reports nothing after it.
The stub is a design choice rather than a missing dependency: ruzstd 0.9 ships an
encoder (`ruzstd::encoding`) this crate has not adopted, and the C library would
need a C-to-WASM toolchain. Wiring up either one changes what the wasm build does
and is a change of its own; this commit only stops the absent encoder from killing
the suite. No fallback exists in this crate — `pipeline` propagates
`CompressFailed` for `ColumnCodec::Zstd`. The stub's own comments said it was a
fallback that encodes "a minimal Zstd frame", which it never did; they now describe
the stub.
They are gated with `#[cfg_attr(target_arch = "wasm32", ignore = "…")]` rather
than `#[cfg(not(target_arch = "wasm32"))]`. The attribute keeps them compiled and
listed by `--list` on that target, so the gap is visible in the run instead of
absent from it, and they keep running natively.
Red arm, with the four tests other changes own skipped:
$ cargo test --profile ci --target wasm32-wasip1 -p nodedb-codec -- <4 skips>
test zstd_codec::tests::better_ratio_than_lz4 ... Error: failed to run main module
exit 134
Green arm, same command:
test result: ok. 252 passed; 0 failed; 8 ignored; 0 measured; 4 filtered out
Each ignored test prints its reason:
test zstd_codec::tests::empty_data ... ignored, nodedb-codec has no wasm encoder: encode() returns CompressFailed
Native is unchanged at 263 passed, 0 failed, 0 ignored. The module's other four
tests stay live on both targets: three are decode-side, and
`streaming_input_limit_precedes_buffer_growth` drives the encoder's declared-limit
check, which rejects before any encoder work.
Why an in-source ignore and not a `--skip` list in the consumer: the gap is
categorical — the target has no encoder at all, not a threshold that moves — and
a named skip in someone else's workflow records it as a number rather than as a
property of this crate.
fmt clean; clippy clean for this crate on native and on `wasm32-wasip1`.
Contributor
Author
|
Closing. This adds #[cfg_attr(target_arch = "wasm32", ignore = …)] to eight zstd encoder tests in nodedb-codec. NodeDB is a server, does not build for wasm32, and will not support wasm. The consumer that needed this is NodeDB Lite's shared-crates-wasip1 job. wasm gating belongs there, not here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Gate the eight zstd encoder tests in
nodedb-codec/src/zstd_codec.rswith#[cfg_attr(target_arch = "wasm32", ignore = "...")]. Test attributes andcomments only; no behaviour change — the wasm arm still returns the same error.
Why
nodedb-codecalready splits the codec by target: C libzstd off wasm, ruzstd onit (
nodedb-codec/Cargo.toml:31-36), andcompress_native's wasm arm is anexplicit stub returning
CompressFailed(nodedb-codec/src/zstd_codec.rs:312-320);no fallback sits under it, and
pipelinepropagates that error forColumnCodec::Zstd. The eight tests callencode/encode_with_level, so they panic —and on
wasm32-wasip1a panic aborts the process: the suite dies at the firstone and reports nothing after it. That makes the crate's wasip1 suite unusable,
which is what a shared-crate wasm job for NodeDB-Lite needs.
That stub is a design choice, not a missing dependency: ruzstd 0.9 ships an
encoder (
ruzstd::encoding—pub mod encodinginlib.rs,compressinencoding/mod.rs,FrameCompressordefined inencoding/frame_compressor.rsandre-exported from
encoding/mod.rs) that this crate has not adopted, and the Clibrary would need a C-to-WASM toolchain. Adopting either one changes what the wasm
build does, so it is deliberately not part of this change. The stub's own comments
used to call it a fallback that encodes "a minimal Zstd frame" — it never did, and
they now describe the stub.
cfg_attr(ignore)keeps them compiled and listed by--liston that target, sothe gap shows up in the run.
cfg(not(target_arch = "wasm32"))would delete themfrom the target instead, and a
--skiplist in a consumer workflow would recordthe gap as a number in someone else's file.
How to check it
Red arm: with the ignores removed and the same four skips, the same command
aborts —
Each ignored test names its reason:
cargo fmt --all -- --checkexits 0;cargo clippy -p nodedb-codec --all-targets -- -D warningsexits 0 natively and for--target wasm32-wasip1.Scope
and
streaming_input_limit_precedes_buffer_growthdrives the encoder'sdeclared-limit check, which rejects before any encoder work.
--skipflags above name tests owned elsewhere: three abort through a32-bit overflow (
delta::tests::rejects_overflowed_noncanonical_and_huge_input_before_allocation,double_delta::tests::huge_count_and_noncanonical_tail_are_rejected_before_decode_work,fastlanes::codec::tests::hostile_counts_and_block_shapes_fail_before_allocation_or_looping)and are fixed by an open change of their own;
vector_quant::opq::tests::top1_recall_on_training_setis a recall thresholdthe target misses and stays a named skip in the consumer job, not here.
wasm job that belongs in the NodeDB-Lite repository.